test: secrets non-leak invariant for logs and summary JSON by clean6378-max-it · Pull Request #96 · cppalliance/local-ci-test-system

clean6378-max-it · 2026-07-29T23:02:04Z

Closes #91

Argv and display() were already covered. This adds tests that configured secret plaintext never shows up in stdout, stderr, the job log, or saved ExecutionSummary JSON.

Helpers in test_executor.py now scan arbitrary strings and files; test_secrets_non_leak.py mocks Popen for the capture path and for a failed run where error_message lands in JSON (to_dict() omits stdout/stderr).

Env injection is out of scope here (#58).

pytest -q tests/test_secrets_non_leak.py tests/test_executor.py -k secret passes.

Summary by CodeRabbit

Tests
- Added shared invariant checks to ensure configured secret plaintext never appears in captured stdout/stderr, on-disk logs, command display output, or persisted execution summary JSON (including failure-path error messages).
- Strengthened argv secret-leak detection by asserting absence of the entire secret set within captured argv text representations and explicitly forbidding --secret / --secret= arguments.
- Expanded end-to-end coverage across success and failure scenarios, including tighter verification of serialized execution summary fields.

coderabbitai · 2026-07-29T23:02:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a37b87ab-cbcb-44eb-9c8a-cf933c485502

📥 Commits

Reviewing files that changed from the base of the PR and between 0f07e29 and fe3ccd2.

📒 Files selected for processing (1)

cli/tests/test_secrets_non_leak.py

📝 Walkthrough

Walkthrough

The changes generalize secret-leak test helpers and add mocked executor tests covering secret absence from process output, log files, command display text, error messages, and persisted execution summaries.

Changes

Secret non-leak coverage

Layer / File(s)	Summary
Generalized secret-leak assertions `cli/tests/test_executor.py`	Adds reusable checks for secret plaintext in arbitrary text and files, and updates argv validation to retain forbidden secret argument checks.
End-to-end capture invariant tests `cli/tests/test_secrets_non_leak.py`	Adds deterministic mocked process tests that verify expected stdout/stderr content is captured while the configured secret is absent from streams, logs, command display text, error messages, and saved `ExecutionSummary` JSON.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: bradjin8, wpak-ai

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the new secrets non-leak tests for logs and execution-summary JSON.
Linked Issues check	✅ Passed	The PR covers the requested invariant across stdout, stderr, logs, display, and JSON, and defers env scope via `#58`.
Out of Scope Changes check	✅ Passed	The changes stay within test coverage for secret non-leak behavior and don't add unrelated functionality.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch test/secrets-non-leak-invariant

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cli/tests/test_secrets_non_leak.py`:
- Around line 54-58: Update both _mock_popen_factory fixtures in
cli/tests/test_secrets_non_leak.py at lines 54-58 and 96-100 to include
INVARIANT_SECRET in the successful stdout/stderr and failed stderr streams.
Extend the related assertions to verify the secret is redacted from captured
output, logs, error_message, and JSON results.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c63d85b-f95e-4864-90f5-7d59976f3944

📥 Commits

Reviewing files that changed from the base of the PR and between a0786a2 and 5956717.

📒 Files selected for processing (2)

cli/tests/test_executor.py
cli/tests/test_secrets_non_leak.py

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cli/tests/test_secrets_non_leak.py (1)

76-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that the mocked streams are actually captured.

These tests only assert that stdout and stderr do not contain the secret. If capture regresses and both streams become empty, the tests still pass. Assert known mock-output fragments before the non-leak checks.

Proposed assertions

         with patch(
             "localci.core.executor.subprocess.Popen",
             return_value=mock_process,
         ):
@@
         assert exit_code == 0
+        assert "compile ok" in stdout
+        assert "deprecated flag" in stderr
         _assert_text_has_no_secret_plaintext(stdout, INVARIANT_SECRET)
         _assert_text_has_no_secret_plaintext(stderr, INVARIANT_SECRET)

         assert result.status == JobStatus.PASSED
         assert result.log_file is not None
+        assert "compile ok" in result.stdout
+        assert "deprecated flag" in result.stderr
         _assert_text_has_no_secret_plaintext(result.stdout, INVARIANT_SECRET)
         _assert_text_has_no_secret_plaintext(result.stderr, INVARIANT_SECRET)

Also applies to: 94-98, 114-118, 134-138

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/tests/test_secrets_non_leak.py` around lines 76 - 80, Update the tests
using _mock_popen_factory to first assert that captured stdout and stderr
contain the known mock-output fragments, such as “compile ok” and “warning:
deprecated flag,” before checking that the secret is absent. Apply the same
capture assertions to the corresponding test cases at the other indicated stream
setups.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cli/tests/test_secrets_non_leak.py`:
- Around line 76-80: Update the tests using _mock_popen_factory to first assert
that captured stdout and stderr contain the known mock-output fragments, such as
“compile ok” and “warning: deprecated flag,” before checking that the secret is
absent. Apply the same capture assertions to the corresponding test cases at the
other indicated stream setups.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7aa53414-5990-4e13-812c-9d0f5ff6cb4e

📥 Commits

Reviewing files that changed from the base of the PR and between a93120b and 0f07e29.

📒 Files selected for processing (1)

cli/tests/test_secrets_non_leak.py

test: assert secrets never leak into job logs or summary JSON

5956717

clean6378-max-it requested review from bradjin8 and wpak-ai as code owners July 29, 2026 23:02

clean6378-max-it self-assigned this Jul 29, 2026

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread cli/tests/test_secrets_non_leak.py

bradjin8 requested changes Jul 30, 2026

View reviewed changes

Comment thread cli/tests/test_secrets_non_leak.py

Comment thread cli/tests/test_secrets_non_leak.py

Comment thread cli/tests/test_secrets_non_leak.py

Comment thread cli/tests/test_secrets_non_leak.py

Comment thread cli/tests/test_executor.py

address Brad's feedback

a93120b

clean6378-max-it requested a review from bradjin8 July 30, 2026 07:39

bradjin8 approved these changes Jul 30, 2026

View reviewed changes

wpak-ai requested changes Jul 30, 2026

View reviewed changes

Comment thread cli/tests/test_secrets_non_leak.py

Address Will's feedbac

0f07e29

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

test: assert captured mock output before secret leak checks

fe3ccd2

wpak-ai approved these changes Jul 30, 2026

View reviewed changes

wpak-ai merged commit b94701e into develop Jul 30, 2026
7 checks passed

wpak-ai deleted the test/secrets-non-leak-invariant branch July 30, 2026 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: secrets non-leak invariant for logs and summary JSON - #96

test: secrets non-leak invariant for logs and summary JSON#96
wpak-ai merged 4 commits into
developfrom
test/secrets-non-leak-invariant

clean6378-max-it commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

clean6378-max-it commented Jul 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clean6378-max-it commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading